docs(container): warn about K8s init-container tag mismatch + RollingUpdate hazard on shared data volumes (refs #9940)#10014
Open
asheshv wants to merge 1 commit into
Conversation
WalkthroughAdds a Kubernetes section describing safe sharing of /var/lib/pgadmin via init containers, requiring init/sidecar image tags not be newer than the main pgAdmin image and recommending ChangesKubernetes Data Volume Sharing Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…umes Refs pgadmin-org#9940. Issue pgadmin-org#9940 reported a SQLite "no such column: sharedserver.db_res" error on a Kubernetes deployment where the main pgAdmin container was pinned to 9.13 but an init container (running `setup.py load-servers` to import a servers.json) used the `pgadmin4:latest` image. The init container's migrations advanced the schema past 9.13's ORM expectations — specifically the `sharedserver_feature_parity_` migration (introduced in 9.15 via PR pgadmin-org#9835) drops the `db_res` column from `sharedserver`. This is a real hazard of pgAdmin's forward-only-with-destructive- operations migration model when combined with the K8s init-container pattern, but it wasn't documented anywhere. Operators hitting this have no guidance on how to avoid it. Add a short section to docs/en_US/container_deployment.rst: - The single rule: init container's image tag must NOT be newer than the main container's, when they share a data volume. - A concrete Helm-values example showing the pinned tags side by side, with an explicit "Do NOT use :latest" note. - A reminder that during upgrades, every reference to the image tag must move together. Pure docs-only change — no code touched. Reviewable on its own.
asheshv
force-pushed
the
docs/k8s-init-container-tag-pinning
branch
from
b02627c to
a3f998f
Compare
asheshv
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en_US/container_deployment.rst`:
- Around line 593-600: Update the examples so Helm-values keys are not mixed
with raw Deployment fields: either split the shown snippets into two labeled
blocks (one demonstrating Helm values using keys like image.registry,
image.repository, image.tag and strategy.type in values.yaml, and a separate raw
Deployment manifest showing image: docker.io/dpage/pgadmin4:9.14,
spec.strategy.type, and extraInitContainers), or replace the current examples
with Helm-values-native keys only (remove Deployment-style fields such as image:
docker.io/dpage/pgadmin4:9.14 and spec.strategy.type from the values example and
instead show image.registry/repository/tag and strategy.type). Ensure the same
change is applied to the other occurrence noted (the snippet around
spec.strategy.type).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f971ea4-af50-42de-b36b-fef4b8d523c1
📒 Files selected for processing (1)
docs/en_US/container_deployment.rst
Comment on lines
+593
to
+600
| # In your Helm values / Deployment spec: | ||
| image: docker.io/dpage/pgadmin4:9.14 # main pgAdmin container | ||
| ... | ||
| extraInitContainers: | ||
| - name: import-servers-json | ||
| # Match the main container's tag exactly. Do NOT use :latest. | ||
| image: docker.io/dpage/pgadmin4:9.14 | ||
| # ... rest of the init container spec ... |
There was a problem hiding this comment.
Clarify Helm values vs Deployment-spec YAML to avoid invalid chart configuration.
The examples at Line 593 and Line 634 are labeled as “Helm values / Deployment spec” but use Deployment-style fields (image: ..., spec.strategy.type). In this repo’s Helm chart, image and strategy come from values keys (image.registry/repository/tag and strategy.type), so readers may paste invalid values into Helm. Please split into two explicit snippets (Helm values vs raw Deployment manifest) or use Helm-values-native keys only.
Also applies to: 634-637
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/en_US/container_deployment.rst` around lines 593 - 600, Update the
examples so Helm-values keys are not mixed with raw Deployment fields: either
split the shown snippets into two labeled blocks (one demonstrating Helm values
using keys like image.registry, image.repository, image.tag and strategy.type in
values.yaml, and a separate raw Deployment manifest showing image:
docker.io/dpage/pgadmin4:9.14, spec.strategy.type, and extraInitContainers), or
replace the current examples with Helm-values-native keys only (remove
Deployment-style fields such as image: docker.io/dpage/pgadmin4:9.14 and
spec.strategy.type from the values example and instead show
image.registry/repository/tag and strategy.type). Ensure the same change is
applied to the other occurrence noted (the snippet around spec.strategy.type).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #9940.
Why
Issue #9940 reported
(sqlite3.OperationalError) no such column: sharedserver.db_resfrom a Kubernetes pgAdmin deployment where the main container and an init container ran on different image tags but shared/var/lib/pgadmin. Root cause: PR #9835 (Apr 2026, ships in 9.15) added a migration that dropsdb_resfromsharedserver. When the init container ranpgadmin4:latest(had the drop migration) and the main container ran9.13(didn't), the init container advanced the schema past 9.13's ORM and the main container'sSELECT sharedserver.db_res …queries started failing.Discussion on the issue exposed a second, related hazard the operator hadn't asked about: even with both image tags pinned correctly, a K8s rolling upgrade across pgAdmin versions is unsafe when old and new pods share a data volume. During the
RollingUpdateoverlap window, the new pod migrates the schema while the old pod is still serving traffic — destructive migrations (column drops, table renames, etc.) immediately break the still-alive old pod's queries.Both hazards are real, both are operator-side configuration concerns, and neither is documented anywhere.
What this PR does
Adds a section to
docs/en_US/container_deployment.rst(after the existing Traefik section, at the end of the file) titled "Sharing the data volume across containers (Kubernetes init containers)", with two subsections:Init-container tag pinning — the rule: the init container's tag must not be newer than the main container's. Example: Helm values showing both pinned to the same version, with an explicit "Do NOT use
:latest" note. Why: pgAdmin migrations are forward-only with destructive operations.Use
strategy: Recreate, notRollingUpdate— the rule: deployment strategy must beRecreate. Why: the rolling-update overlap window between old and new pods sharing a data volume is unsafe with single-step destructive migrations (no expand-contract pattern); the only safe path is to kill the old pod before the new one migrates. Trade-off: a few seconds of downtime per upgrade, in exchange for not serving traffic from a pod whose ORM is misaligned with the database schema underneath it.What this PR doesn't do
This is a docs-only change — no code, no migration changes, no behavior change. The reporter on #9940 has a concrete workaround (pin the init container tag and switch to
Recreate); this PR ensures future operators don't hit either trap without warning.A longer-term, code-side improvement would be a startup-time
alembic_versionmismatch check: at pgAdmin main-container boot, refuse to start with a clear error if the DB has been migrated by a newer pgAdmin than the running code. That's its own design discussion and out of scope here.A still-longer-term option — adopting the expand-contract migration pattern so destructive operations span multiple releases and rolling upgrades become safe — would be a substantial design change to pgAdmin's migration policy, also out of scope.
Stats
Test plan
docutils.core.publish_doctreeparses without new warnings/errorsmake docs(Sphinx HTML build) and confirm both subsections render correctly indocs/en_US/_build/html/container_deployment.html.